Skip to content

Conversation

@A-312
Copy link
Contributor

@A-312 A-312 commented Oct 27, 2019

Fix: #312

Q/A :

Add format with regex name.

@A-312 A-312 changed the title addFormat(name, validate) accepts regExp for name addFormat(name, validate) accepts regex for name Oct 27, 2019
@A-312
Copy link
Contributor Author

A-312 commented Dec 7, 2019

I forgot to add documentation in README 😢

@madarche
Copy link
Collaborator

Thank you a lot for this PR. But sorry I think this brings complexity in the convict code, while providing not much added value. The same functionality provided by this PR can be achieved by registering multiple formats, which can be done by a custom module/lib.

My main objectives with convict is to keep it safe and as simple as possible to use and to maintain.

@madarche madarche closed this Dec 13, 2019
@A-312
Copy link
Contributor Author

A-312 commented Dec 14, 2019

My main objectives with convict is to keep it safe and as simple as possible to use and to maintain.

I only add two loops enabled when types['>RegExp']; is defined (= when a regex name format is used). I used maintainable function, I mean old function and old ES standard. I didn't see the not maintainable side.

Add 10 formats then 1 is enough, is not maintainable on my side. I prefere to code dynamically (add a loop) instead copy/paste the same line/function.

And this PR allow list format, whitout copy/paste 10 formats. Also I can make a hack like this :

    const schema = {
      dependencies: {
        format: 'list',
        subformat: 'string', //instead format: Array[String]
        default: []
      },

but i think format like 'Array[String]' is more readable than format & subformat key.

I think also you can close #342 because is the same way to do. On my eye, this PR open the same opening that 342 but for format.

I replaced (in 342) existing getters functions, I mean : importArguments, importEnvironment, addDefaultValues. By only one function which works dynamically. Now people will choose how they can get value or import arg/env variables.


And i would like to know. Did you refuse because you have less time than before for convict ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add regex at name for addFormat ?

2 participants